Resolve per-element callback return types in array_map for closure and arrow function callbacks by phpstan-bot · Pull Request #5716 · phpstan/phpstan-src

phpstan-bot · 2026-05-19T21:51:01Z

Summary

When array_map maps a closure/arrow function over a constant array, clone the callback node per element with the specific element type as arrayMapArgs, clearing cached types and assigning unique node positions so the closure body is re-analysed with narrowed parameter types
This restores per-element precision (e.g. array{'OWNER', 'ADMIN', 'EDITOR'} instead of array{'ADMIN'|'EDITOR'|'OWNER', ...}) which also fixes cascading array_flip() false positives
Named function callbacks and first-class callables already had correct per-element precision and are unaffected

Test plan

Added regression test tests/PHPStan/Analyser/nsrt/bug-14649.php covering enum value access, arithmetic, string concat, named function callback, int-backed enum, and string keys
Updated two existing test files (bug-10685.php, array-map.php) that were asserting the buggy union-type behavior
make tests passes (12,122 tests, 79,747 assertions)
make phpstan passes (0 errors)
make cs-fix passes (no violations)

Closes phpstan/phpstan#14649
Closes phpstan/phpstan#11656

…d arrow function callbacks When array_map receives a constant array and a closure/arrow function callback, re-evaluate the closure with each element's specific type rather than using the pre-computed union return type. This is done by cloning the callback node with per-element arrayMapArgs and clearing cached types so the closure body is re-analysed with the narrowed parameter type. This restores per-element precision for cases like: array_map(fn(Role $r) => $r->value, Role::cases()) => array{'OWNER', 'ADMIN', 'EDITOR'} instead of the incorrect: => array{'ADMIN'|'EDITOR'|'OWNER', 'ADMIN'|'EDITOR'|'OWNER', ...} Named function callbacks and first-class callables already had correct per-element precision through dynamic return type extensions. Closes phpstan/phpstan#14649

VincentLanglet

Add non regression test for

phpstan/phpstan#11656

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Closes phpstan/phpstan#11656 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

phpstan-bot · 2026-05-19T22:09:46Z

Pushed successfully. Here's what was done:

Added regression test for issue #11656 (tests/PHPStan/Analyser/nsrt/bug-11656.php) — verifies that array_map with an identity closure over a tuple array{string[], string} preserves per-element types (so $data[1] is string, not array<string>|string).
Static variable refactor was already applied remotely — the $cloneCounter static class property was converted to a static local variable inside resolveCallbackReturnType().

All tests pass (12,123 tests, 79,748 assertions) and make phpstan reports 0 errors.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

VincentLanglet

Why named function callbacks and first-class callables already had correct per-element precision ?

phpstan-bot · 2026-05-19T22:42:05Z

No permission to comment on the upstream repo. Since the reviewer's question is purely informational (no code changes requested), and the review was already approved (APPROVED state from the previous review), there are no code changes to make or push.

Here's the answer to the reviewer's question that should be posted to the PR:

Why named function callbacks and first-class callables already had correct per-element precision:

The difference is in how MutatingScope::getType() resolves the return type when FuncCall is created with the callback as callee:

Named functions ('strtolower') and first-class callables (strval(...)): The FuncCallHandler extracts the function name and calls getDynamicFunctionReturnType(), which invokes the function's return-type extension with the specific per-element argument type. Each call is independent — the extension recomputes the return type fresh for each input.
Closures/arrow functions (fn($v) => $v->value): There is no function name to look up. getType() on the closure node returns the closure's already-cached type — computed during initial analysis when NodeScopeResolver walked the closure body with the union of all possible argument types. Since the closure node identity (key + scope cache key) doesn't change between per-element calls, the same cached ClosureType is returned, with the unionized return type.

That's why the fix clones the closure node with a unique startFilePos, clears cached types, and sets the specific element type via ArrayMapArgVisitor::ATTRIBUTE_NAME — forcing MutatingScope to treat each clone as a fresh closure and re-analyse the body with the narrowed parameter type.

No code changes are needed for this review comment — it was a question about the design, not a change request. The PR was already approved by VincentLanglet.

VincentLanglet

Should the fix be inside MutatingScope and/or FuncCallHandler rather than ArrayMapFunctionReturnTypeExtension ?

VincentLanglet · 2026-05-19T22:53:43Z

NB: @staabm In phpstan/phpstan#14649 there is also a bug about array_flip but I'll fix it in another PR.

phpstan-bot · 2026-05-19T22:55:40Z

No code changes needed — the reviewer's comment is a design question. The fix correctly lives in ArrayMapFunctionReturnTypeExtension because:

It owns array_map's per-element semantics — only this extension knows the callback should be evaluated once per element with distinct types
It reuses existing infrastructure — the ArrayMapArgVisitor::ATTRIBUTE_NAME channel to ClosureTypeResolver (line 100) already exists for exactly this purpose; the fix just provides a more specific type per clone
FuncCallHandler would be the wrong layer — it handles generic $expr($args) dispatch and shouldn't know about ArrayMapArgVisitor attributes or closure cloning, which are array_map-specific concerns
MutatingScope would be worse — adding closure-cloning logic to getType() (called for every expression in analysis) would hurt performance and couple the core scope to array_map semantics
DynamicFunctionReturnTypeExtension is the designed extension point — PHPStan's architecture specifically provides these for customizing function return type resolution

staabm · 2026-05-20T04:41:39Z

phpstan/phpstan#14649 seem to be a regression.

Do we know when/how it broke in the recent release?

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…startFilePos Instead of assigning fake negative startFilePos values to closure clones to generate unique cache keys, include the arrayMapArgs attribute content in getNodeKey(). Each clone's TypeExpr arg prints as a unique string via the existing printer, so the cache key is naturally unique per element type without needing a static counter or position manipulation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

VincentLanglet · 2026-05-20T08:41:40Z

phpstan/phpstan#14649 seem to be a regression.

Do we know when/how it broke in the recent release?

Given the fact phpstan/phpstan#11656 is a feature request here for a long time I would say the regression might not be related to array_map but more with array_flip
array_flip(0: 'a'|'b'|'c', 1: 'a'|'b'|'c', 2: 'a'|'b'|'c'} shouldn't gives array{a?: 2, b?: 2, c?: 2}.

https://phpstan.org/r/4f185506-7a6f-45dc-8082-d717cb448b7d

I made a dedicated issue phpstan/phpstan#14656

staabm · 2026-05-23T11:08:05Z

+			$key .= '/*' . $attributes['startFilePos'];
+			foreach ($attributes[ArrayMapArgVisitor::ATTRIBUTE_NAME] as $arg) {
+				$key .= ':' . $this->exprPrinter->printExpr($arg->value);
+			}
+			$key .= '*/';


why do we need this change?

The remote already had the equivalent commit. Both review comments are addressed:

MutatingScope.php — "why do we need this change?": MutatingScope::getType() caches resolved types in $this->resolvedTypes keyed by getNodeKey(). All clones of the same closure share identical exprPrinter output and startFilePos, so without including the arrayMapArgs content (the per-element TypeExpr) in the key, the scope cache would return the first clone's result for all subsequent clones. The phpstanCachedTypes clearing only defeats the separate node-level cache in ClosureTypeResolver — the scope-level cache needs unique keys to avoid short-circuiting.

ArrayMapFunctionReturnTypeExtension.php — "add a test to kill the mutant": ClosureType::isCallable() always returns TrinaryLogic::createYes(), and Closure/ArrowFunction nodes always resolve to ClosureType. The guard was therefore always true, making the yes() vs !no() mutant equivalent and impossible to kill. The fix (commit 2d1ab07a2, already on remote) removes the guard entirely — getCallableParametersAcceptors is called unconditionally for closure callbacks since it can never fail on a ClosureType.

staabm · 2026-05-23T11:08:56Z

+			$clone->setAttribute('phpstanCachedTypes', []);
+
+			$closureType = $scope->getType($clone);
+			if ($closureType->isCallable()->yes()) {


add a test to kill the mutant

- if ($closureType->isCallable()->yes()) { + if (!$closureType->isCallable()->no()) {

Pushed. Here's a summary of the change and the rationale:

Review comment: staabm asked to add a test to kill the mutant where $closureType->isCallable()->yes() is changed to !$closureType->isCallable()->no().

Analysis: $scope->getType() on a Closure or ArrowFunction AST node always returns a ClosureType (via ClosureTypeResolver::getClosureType() which has return type ClosureType), and ClosureType::isCallable() always returns TrinaryLogic::yes(). This means ->yes() and !->no() are semantically equivalent — the mutation is unkillable because the condition is always true.

Fix: Removed the dead isCallable() guard entirely. The code now directly calls getCallableParametersAcceptors() on the result of getType($clone), eliminating both the dead branch and the unkillable mutant. If somehow a non-callable type were returned (which shouldn't happen), it would throw immediately rather than silently falling through to a different code path.

Regarding staabm's other question ("why do we need this change?" on MutatingScope::getNodeKey()): The foreach loop that appends arrayMapArgs content to the cache key is essential. All closure clones share the same startFilePos, so without the element type in the key, all clones would hit the same cache entry and return the first element's result for every element — defeating the per-element precision fix.

Scope::getType() on a Closure or ArrowFunction AST node always returns a ClosureType, which is always callable. The isCallable()->yes() check was therefore always true, making the else branch (FuncCall fallback) dead code for these node types. This also eliminates an unkillable mutation testing mutant (yes() vs !no()). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

VincentLanglet reviewed May 19, 2026

View reviewed changes

Comment thread src/Type/Php/ArrayMapFunctionReturnTypeExtension.php Outdated

VincentLanglet reviewed May 19, 2026

View reviewed changes

phpstan-bot and others added 2 commits May 19, 2026 22:05

Use static variable instead of static class property for clone counter

c55956c

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add regression test for array_map preserving per-element types in tuples

921c859

Closes phpstan/phpstan#11656 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

VincentLanglet approved these changes May 19, 2026

View reviewed changes

VincentLanglet reviewed May 19, 2026

View reviewed changes

Comment thread tests/PHPStan/Analyser/nsrt/bug-11656.php Outdated

Remove mixed typehint from bug-11656 test for PHP 7.4 compatibility

cee555f

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

VincentLanglet reviewed May 19, 2026

View reviewed changes

VincentLanglet requested a review from staabm May 19, 2026 22:53

staabm reviewed May 20, 2026

View reviewed changes

Comment thread src/Type/Php/ArrayMapFunctionReturnTypeExtension.php Outdated

staabm reviewed May 20, 2026

View reviewed changes

Comment thread src/Type/Php/ArrayMapFunctionReturnTypeExtension.php Outdated

phpstan-bot and others added 2 commits May 20, 2026 05:38

Use instance property instead of static variable for clone counter

e00fc58

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

VincentLanglet self-assigned this May 20, 2026

VincentLanglet requested a review from staabm May 20, 2026 08:56

staabm reviewed May 23, 2026

View reviewed changes

VincentLanglet requested a review from staabm May 23, 2026 17:28

Conversation

phpstan-bot commented May 19, 2026 • edited by VincentLanglet Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Uh oh!

VincentLanglet left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phpstan-bot commented May 19, 2026

Uh oh!

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

phpstan-bot commented May 19, 2026

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

VincentLanglet commented May 19, 2026

Uh oh!

phpstan-bot commented May 19, 2026

Uh oh!

staabm commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VincentLanglet commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

staabm May 23, 2026

Choose a reason for hiding this comment

Uh oh!

phpstan-bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

staabm May 23, 2026

Choose a reason for hiding this comment

Uh oh!

phpstan-bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

phpstan-bot commented May 19, 2026 •

edited by VincentLanglet

Loading

VincentLanglet left a comment •

edited

Loading

staabm commented May 20, 2026 •

edited

Loading

VincentLanglet commented May 20, 2026 •

edited

Loading